fix: implement node:tty #20892

littledivy · 2023-10-12T12:10:18Z

Fixes #21012
Closes #20855

Fixes #20890
Fixes #20611
Fixes #20336
Fixes create-svelte from #17248

Fixes more reports here:

littledivy · 2023-10-12T14:01:23Z

ext/node/polyfills/process.ts

-function makeLazyStream<T>(objectFactory: () => T): T {
-  return new Proxy({}, {
-    get: function (_, prop, receiver) {
-      // deno-lint-ignore no-explicit-any
-      return Reflect.get(objectFactory() as any, prop, receiver);


cc @bartlomieju @mmastrac

This breaks code in _streams.mjs because of the use of Proxy:

divy@mini /tmp> ~/gh/deno/target/debug/deno run -A npm:nuxi@latest init my-app [5:25:30 PM] ERROR 'get' on proxy: property '_eventsCount' is a read-only and non-configurable data property on the proxy target but the proxy did not return its actual value (expected '2' but got '1') at _addListener (ext:deno_node/_stream.mjs:1884:9) at Proxy.addListener (ext:deno_node/_stream.mjs:1913:14) at Proxy.Readable.on (ext:deno_node/_stream.mjs:3275:39) at Proxy.Readable.pipe (ext:deno_node/_stream.mjs:3177:11)

when its trying to update the eventsCount. I reverted it back to not lazy load the stdin/out/err streams which fixed the issue.

ext/node/polyfills/tty.js

bartlomieju

Can you enable some additional compat tests for this PR?

ext/node/polyfills/process.ts

littledivy · 2023-10-12T16:44:00Z

@bartlomieju all tty related compat tests seem to be enabled already cli/tests/unit_node/pseudo-tty

bartlomieju · 2023-10-12T21:52:30Z

@bartlomieju all tty related compat tests seem to be enabled already cli/tests/unit_node/pseudo-tty

I'm not sure - eg. I can see test-set-raw-mode-reset.js in tools/node_compat/node/test/pseudo-tty/ but it's not in cli/tests/node_compat/test/pseudo-tty/. This particular test would be useful for testing the atexit functionality.

cli/util/unix.rs

littledivy · 2023-10-24T09:40:38Z

~~Hanging on lsp_testing_api~~ Fixed

Signed-off-by: Divy Srivastava <[email protected]>

mmastrac

Looks good, some simplification and unsafe removal suggestions.

ext/node/ops/util.rs

mmastrac · 2023-10-29T12:53:27Z

ext/node/ops/util.rs

+
+#[repr(u32)]
+enum HandleType {
+  #[allow(dead_code)]


TODO for later work?

Dead code on Windows for TCP and UDP is TODO.

mmastrac · 2023-10-29T13:00:52Z

/bench startup_nop_trivial

denobot · 2023-10-29T13:00:57Z

startup_nop_trivial

	time	user	system	relative
node	23 ms	16 ms	8 ms	+105.4%
bun	11 ms	4 ms	7 ms	best
deno	16 ms	10 ms	7 ms	+47.6%
deno-baseline	16 ms	11 ms	5 ms	+44.6%

^{start: Sun, 29 Oct 2023 13:00:57 GMT
id: 52ef5c36-3ae8-4ac4-894f-1988a54ccaaf
server: 5645ab74-bf2f-4fe3-86b4-f575d075ce2b}

mmastrac · 2023-10-29T13:07:32Z

Looks like no real startup impact of this change per the benchmark below. Awesome!

mmastrac

LGTM, nice

littledivy · 2023-10-30T05:23:37Z

@mmastrac The early is_terminal check seems to bring back the hang on Windows. I've reverted back.

bartlomieju

LGTM too, try to enable some of the Node compat tests that check if the terminal is restored on exit

benmccann · 2023-10-30T20:49:29Z

Fixes create-svelte from #17248

As a heads up, it was reported that using the latest Deno from main didn't fix this issue: #17248 (comment)

Fixes denoland#21012 Closes denoland#20855 Fixes denoland#20890 Fixes denoland#20611 Fixes denoland#20336 Fixes `create-svelte` from denoland#17248 Fixes more reports here: - denoland#6529 (comment) - denoland#6529 (comment) - denoland#6529 (comment)

implement node:tty

980fb05

littledivy changed the title ~~implement node:tty~~ fix: Implement node:tty Oct 12, 2023

Revert lazy stdin streams

eba9e82

littledivy commented Oct 12, 2023

View reviewed changes

littledivy marked this pull request as ready for review October 12, 2023 14:01

littledivy commented Oct 12, 2023

View reviewed changes

ext/node/polyfills/tty.js Show resolved Hide resolved

Fix lint

c98a5ab

bartlomieju reviewed Oct 12, 2023

View reviewed changes

ext/node/polyfills/process.ts Outdated Show resolved Hide resolved

littledivy added 4 commits October 12, 2023 20:06

Restore stdio on exit

8e8e7e7

x

b871a5a

fix lint windows

3b37419

oopsie

181cddd

littledivy requested a review from bartlomieju October 12, 2023 17:35

bartlomieju reviewed Oct 12, 2023

View reviewed changes

cli/util/unix.rs Show resolved Hide resolved

Merge branch 'main' into node_tty

1496539

littledivy changed the title ~~fix: Implement node:tty~~ fix: implement node:tty Oct 20, 2023

littledivy added 11 commits October 27, 2023 08:58

Merge branch 'main' into node_tty

03cb93b

Signed-off-by: Divy Srivastava <[email protected]>

windows fixes?

cfed35d

Merge branch 'node_tty' of github.com:littledivy/deno into node_tty

2881000

fmt

a268c83

fix js lint

bc16899

macOS fix?

7bf3646

maybe fixes really

362fe08

fmt

9e3e2a6

implement guessHandleType

4099f2a

Merge branch 'node_tty' of github.com:littledivy/deno into node_tty

8a34458

zzz

ad61842

fixes

29db367

littledivy requested a review from bartlomieju October 29, 2023 11:50

mmastrac reviewed Oct 29, 2023

View reviewed changes

littledivy added 2 commits October 29, 2023 19:41

x

2308af9

x

71bae42

mmastrac approved these changes Oct 29, 2023

View reviewed changes

littledivy force-pushed the node_tty branch from 74659d0 to 29db367 Compare October 30, 2023 05:23

bartlomieju approved these changes Oct 30, 2023

View reviewed changes

littledivy added 3 commits October 30, 2023 19:21

add some tests

7c350d3

fix

e9a79cf

x

709b635

littledivy enabled auto-merge (squash) October 30, 2023 14:12

littledivy added 3 commits October 30, 2023 19:44

x

28db64c

x

8896c9c

x

3d56be1

littledivy merged commit 0920410 into denoland:main Oct 30, 2023
13 checks passed

littledivy deleted the node_tty branch October 30, 2023 15:53

benmccann mentioned this pull request Oct 30, 2023

Support for SvelteKit #17248

Open

bartlomieju mentioned this pull request Nov 6, 2023

Terminal is unresponsive after running Vite #17933

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: implement node:tty #20892

fix: implement node:tty #20892

littledivy commented Oct 12, 2023 •

edited

Loading

littledivy Oct 12, 2023

bartlomieju left a comment

littledivy commented Oct 12, 2023

bartlomieju commented Oct 12, 2023

littledivy commented Oct 24, 2023 •

edited

Loading

mmastrac left a comment

mmastrac Oct 29, 2023

littledivy Oct 29, 2023

mmastrac commented Oct 29, 2023

denobot commented Oct 29, 2023 •

edited

Loading

mmastrac commented Oct 29, 2023

mmastrac left a comment

littledivy commented Oct 30, 2023

bartlomieju left a comment

benmccann commented Oct 30, 2023

fix: implement node:tty #20892

fix: implement node:tty #20892

Conversation

littledivy commented Oct 12, 2023 • edited Loading

littledivy Oct 12, 2023

Choose a reason for hiding this comment

bartlomieju left a comment

Choose a reason for hiding this comment

littledivy commented Oct 12, 2023

bartlomieju commented Oct 12, 2023

littledivy commented Oct 24, 2023 • edited Loading

mmastrac left a comment

Choose a reason for hiding this comment

mmastrac Oct 29, 2023

Choose a reason for hiding this comment

littledivy Oct 29, 2023

Choose a reason for hiding this comment

mmastrac commented Oct 29, 2023

denobot commented Oct 29, 2023 • edited Loading

startup_nop_trivial

mmastrac commented Oct 29, 2023

mmastrac left a comment

Choose a reason for hiding this comment

littledivy commented Oct 30, 2023

bartlomieju left a comment

Choose a reason for hiding this comment

benmccann commented Oct 30, 2023

littledivy commented Oct 12, 2023 •

edited

Loading

littledivy commented Oct 24, 2023 •

edited

Loading

denobot commented Oct 29, 2023 •

edited

Loading